Skip to content

ROSAENG-61053 | chore: bump Go to 1.26.3#3321

Open
olucasfreitas wants to merge 2 commits into
openshift:masterfrom
olucasfreitas:ROSAENG-61053-go126
Open

ROSAENG-61053 | chore: bump Go to 1.26.3#3321
olucasfreitas wants to merge 2 commits into
openshift:masterfrom
olucasfreitas:ROSAENG-61053-go126

Conversation

@olucasfreitas

@olucasfreitas olucasfreitas commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Summary

  • bump Go from 1.25.8 to 1.26.3 in go.mod, all Dockerfiles, Renovate config, and the committed Bingo tool modules
  • keep the verified go fix ./... mechanical cleanup that already passes the repo's local gates on this branch
  • follow the same 1.26.3 rollout pattern used in terraform-provider-rhcs#1240 because 1.26.4 is not available in all required images yet

Test plan

  • GOTOOLCHAIN=go1.26.3 go mod tidy
  • GOTOOLCHAIN=go1.26.3 go mod download
  • GOTOOLCHAIN=go1.26.3 go mod verify
  • GOTOOLCHAIN=go1.26.3 go fix ./...
  • make generate
  • make pre-push-checks
  • Prow presubmits for the new commit

Related

Summary by CodeRabbit

  • Chores
    • Updated build/test images and Go version constraints to Go 1.26.3.
    • Adjusted automated dependency update rules to allow broader Go module upgrades where safe.
  • Bug Fixes
    • Standardized several CLI validation and failure messages to consistent lowercase wording.
    • Improved text/table output generation for listing/reporting commands without changing displayed data.
  • Tests
    • Updated assertions to match updated lowercase error-message text.

@openshift-ci openshift-ci Bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 2, 2026
@openshift-ci

openshift-ci Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@coderabbitai

coderabbitai Bot commented Jul 2, 2026

Copy link
Copy Markdown

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: bcce447a-dbe8-4eec-ab57-05fc70881073

📥 Commits

Reviewing files that changed from the base of the PR and between aee8afd and 23ac5e9.

📒 Files selected for processing (30)
  • .bingo/bingo.mod
  • .bingo/gci.mod
  • .bingo/go-bindata.mod
  • .bingo/golangci-lint.mod
  • .bingo/goreleaser.mod
  • .bingo/mockgen.mod
  • Dockerfile
  • cmd/create/iamserviceaccount/cmd.go
  • cmd/create/idp/cmd.go
  • cmd/create/idp/htpasswd.go
  • cmd/edit/ingress/flags.go
  • cmd/list/accessrequests/cmd.go
  • cmd/list/imagemirrors/cmd.go
  • cmd/list/imagemirrors/cmd_test.go
  • go.mod
  • images/Dockerfile.e2e
  • images/Dockerfile.konflux
  • pkg/fedramp/config.go
  • pkg/helper/url/helpers.go
  • pkg/iamserviceaccount/helpers.go
  • pkg/input/helper.go
  • pkg/kubeletconfig/output.go
  • pkg/logforwarding/helpers.go
  • pkg/logforwarding/output.go
  • pkg/logging/aws_logger.go
  • pkg/logging/round_tripper.go
  • pkg/network/helper.go
  • pkg/output/reporter.go
  • pkg/reporter/reporter.go
  • renovate.json
✅ Files skipped from review due to trivial changes (8)
  • .bingo/go-bindata.mod
  • .bingo/gci.mod
  • Dockerfile
  • .bingo/golangci-lint.mod
  • cmd/list/imagemirrors/cmd_test.go
  • .bingo/goreleaser.mod
  • .bingo/mockgen.mod
  • cmd/create/iamserviceaccount/cmd.go
🚧 Files skipped from review as they are similar to previous changes (22)
  • images/Dockerfile.konflux
  • images/Dockerfile.e2e
  • pkg/input/helper.go
  • .bingo/bingo.mod
  • pkg/kubeletconfig/output.go
  • cmd/list/imagemirrors/cmd.go
  • pkg/logging/aws_logger.go
  • renovate.json
  • cmd/create/idp/htpasswd.go
  • cmd/list/accessrequests/cmd.go
  • pkg/logforwarding/output.go
  • pkg/helper/url/helpers.go
  • pkg/logforwarding/helpers.go
  • pkg/fedramp/config.go
  • go.mod
  • cmd/edit/ingress/flags.go
  • cmd/create/idp/cmd.go
  • pkg/network/helper.go
  • pkg/iamserviceaccount/helpers.go
  • pkg/output/reporter.go
  • pkg/logging/round_tripper.go
  • pkg/reporter/reporter.go

📝 Walkthrough

Walkthrough

This PR updates Go version references in module files, Dockerfiles, and Renovate config, and removes one Renovate package rule. It also refactors command and helper code to use any, slices, maps, strings.Cut, and strings.Builder, with several validation and error-message text changes.

Suggested reviewers: gdbranco, robpblake

🚥 Pre-merge checks | ✅ 14 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description covers summary, tests, and related issue, but it omits several required template sections and details. Add the missing template sections, or mark inapplicable items as N/A, and briefly explain the issue, behavior change, and validation.
✅ Passed checks (14 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise, matches the PR's main change, and follows the repository's required ticket/type format.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names ✅ Passed Changed Ginkgo titles in the diff are all static/descriptive; none include names, UUIDs, dates, IPs, or other run-specific data.
Test Structure And Quality ✅ Passed Changed Ginkgo tests are small unit/e2e cases; each It covers one behavior, uses existing BeforeEach/AfterEach patterns, and adds no new Eventually/cleanup risks.
Microshift Test Compatibility ✅ Passed The PR only changes toolchain/Dockerfile/config files; no new Ginkgo e2e specs were added, so MicroShift compatibility is not implicated.
Single Node Openshift (Sno) Test Compatibility ✅ Passed No new or changed Ginkgo e2e specs were added; the diff only touches Dockerfiles, mod files, and helper/CLI code, with no SNO-sensitive test patterns found.
Topology-Aware Scheduling Compatibility ✅ Passed Only Go/tooling version bumps and mechanical refactors changed; no deployment manifests, controllers, or scheduling constraints were added or modified.
Ote Binary Stdout Contract ✅ Passed PASS: The PR only changes toolchain/config files; no process-level Go code or stdout writes (fmt.Print/klog/TestMain/RunSpecs) were added.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed No new Ginkgo e2e tests were added; the only modified test is a unit test using mocked cluster APIs, with no IPv4 or external-connectivity assumptions.
No-Weak-Crypto ✅ Passed Changed files show no MD5/SHA1/DES/RC4/3DES/Blowfish/ECB or custom crypto; the only new comparison is strings.EqualFold on a public IDP name, not a secret/token.
Container-Privileges ✅ Passed No privileged=true, hostPID/Network/IPC, SYS_ADMIN, or allowPrivilegeEscalation changes; Dockerfile USER root lines are preexisting build-stage setup.
No-Sensitive-Data-In-Logs ✅ Passed Only toolchain/config files changed; no logging statements or secret-like strings were added in the diff.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@openshift-ci

openshift-ci Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: olucasfreitas

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 2, 2026
@olucasfreitas olucasfreitas marked this pull request as ready for review July 2, 2026 13:52
@openshift-ci openshift-ci Bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 2, 2026
@openshift-ci openshift-ci Bot requested review from amandahla and marcolan018 July 2, 2026 13:53
@amandahla

Copy link
Copy Markdown
Contributor

Hi @olucasfreitas this might be a good opportunity to also run the go fix command wdyt

@olucasfreitas

Copy link
Copy Markdown
Contributor Author

@amandahla agreed, will do it here and ping you again

@olucasfreitas

Copy link
Copy Markdown
Contributor Author

@amandahla added in 862952638.

I ran go fix ./... for the Go 1.26 upgrade, regenerated bindata, and kept the mechanical subset that passes the repo gates locally. Current local verification on this branch is green:

  • fmt-check
  • build
  • lint
  • changed-lines coverage
  • unit and integration tests

I left the PR as draft until the paired release PR lands and Prow can pick up the Go 1.26 builder.

@olucasfreitas olucasfreitas force-pushed the ROSAENG-61053-go126 branch from 8629526 to 9e73470 Compare July 2, 2026 18:36

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@cmd/create/idp/cmd.go`:
- Around line 447-461: The IDP validation messages in ValidateIdpName and
getMappingMethod no longer match the existing e2e assertions, so align the
returned error strings with the expected wording. Update the cluster-admin
reserve message in ValidateIdpName and the mapping method validation message in
getMappingMethod to use the same capitalization and punctuation as the tests, or
adjust the corresponding e2e assertions to match the new strings.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: c734004f-91cb-47af-a877-4371e66bae88

📥 Commits

Reviewing files that changed from the base of the PR and between 6c9db38 and 8629526.

📒 Files selected for processing (19)
  • cmd/create/iamserviceaccount/cmd.go
  • cmd/create/idp/cmd.go
  • cmd/create/idp/htpasswd.go
  • cmd/edit/ingress/flags.go
  • cmd/list/accessrequests/cmd.go
  • cmd/list/imagemirrors/cmd.go
  • cmd/list/imagemirrors/cmd_test.go
  • pkg/fedramp/config.go
  • pkg/helper/url/helpers.go
  • pkg/iamserviceaccount/helpers.go
  • pkg/input/helper.go
  • pkg/kubeletconfig/output.go
  • pkg/logforwarding/helpers.go
  • pkg/logforwarding/output.go
  • pkg/logging/aws_logger.go
  • pkg/logging/round_tripper.go
  • pkg/network/helper.go
  • pkg/output/reporter.go
  • pkg/reporter/reporter.go
✅ Files skipped from review due to trivial changes (5)
  • pkg/logforwarding/output.go
  • cmd/list/imagemirrors/cmd_test.go
  • cmd/create/idp/htpasswd.go
  • pkg/logging/round_tripper.go
  • cmd/create/iamserviceaccount/cmd.go

Comment thread cmd/create/idp/cmd.go
Comment on lines +447 to 461
func ValidateIdpName(idpName any) error {

name, ok := idpName.(string)

if !ok {
return fmt.Errorf("Invalid type for identity provider name. Expected a string, got %T", idpName)
return fmt.Errorf("invalid type for identity provider name: expected a string, got %T", idpName)
}

if !idRE.MatchString(name) {
return fmt.Errorf("Invalid identifier '%s' for 'name'", idpName)
return fmt.Errorf("invalid identifier '%s' for 'name'", idpName)
}

if strings.EqualFold(name, "cluster-admin") {
return fmt.Errorf("The name \"cluster-admin\" is reserved for admin user IDP")
return fmt.Errorf("the name \"cluster-admin\" is reserved for admin user IDP")
}

@coderabbitai coderabbitai Bot Jul 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Check if e2e tests were updated to match new error wording.
rg -n "reserved for admin user IDP|Expected a valid mapping method" tests/e2e/

Repository: openshift/rosa

Length of output: 450


Keep the IDP error strings aligned with the existing e2e assertions.
ValidateIdpName now returns the name "cluster-admin" is reserved for admin user IDP, and getMappingMethod now says expected a valid mapping method; options are %s. Existing e2e tests still look for The name "cluster-admin" is reserved for admin user IDP and Expected a valid mapping method. Options are [add claim generate lookup], so this wording change will break those checks unless the tests are updated too.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cmd/create/idp/cmd.go` around lines 447 - 461, The IDP validation messages in
ValidateIdpName and getMappingMethod no longer match the existing e2e
assertions, so align the returned error strings with the expected wording.
Update the cluster-admin reserve message in ValidateIdpName and the mapping
method validation message in getMappingMethod to use the same capitalization and
punctuation as the tests, or adjust the corresponding e2e assertions to match
the new strings.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@olucasfreitas seems valid wdyt

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
rg -n "reserved for admin user IDP|Expected a valid mapping method|expected a valid mapping method" -C2 --iglob '*.go' .

Length of output: 3759


Confirmed: tests/e2e/test_rosacli_idp.go (Lines 514 and 815) still asserts the old wording (Expected a valid mapping method. Options are [...] and The name "cluster-admin" is reserved for admin user IDP), which will break once cmd/create/idp/cmd.go emits the new lower-case strings. This needs to be fixed in one of these files before merging.

@amandahla Confirmed — the e2e tests still expect the old wording:

  • tests/e2e/test_rosacli_idp.go Line 514: "Failed to create IDP for cluster '%s': Expected a valid mapping method. Options are [add claim generate lookup]"
  • tests/e2e/test_rosacli_idp.go Line 815: Expect(textData).Should(ContainSubstring("The name \"cluster-admin\" is reserved for admin user IDP"))

Since cmd/create/idp/cmd.go now returns:

  • "expected a valid mapping method; options are %s" (Line 553)
  • "the name \"cluster-admin\" is reserved for admin user IDP" (Line 460)

these e2e assertions will fail unless updated. You'll need to update the two tests/e2e/test_rosacli_idp.go assertions to match the new lower-case wording (or revert the wording change in cmd.go if you prefer to keep the old strings for consistency with e2e expectations).


🧬 Code Graph Analysis Results

Possibly relevant snippet(s)

pkg/output/reporter.go (lines 39-45)

func (r *StructuredReporter) Errorf(format string, args ...any) error {
	err := fmt.Errorf(format, args...)
	if !PrintError(err) {
		return r.inner.Errorf(format, args...)
	}
	return err
}

pkg/output/reporter.go (lines 47-51)

func (r *StructuredReporter) Warnf(format string, args ...any) {
	if !PrintWarn(fmt.Errorf(format, args...)) {
		r.inner.Warnf(format, args...)
	}
}

pkg/output/reporter.go (lines 53-55)

func (r *StructuredReporter) Debugf(format string, args ...any) {
	r.inner.Debugf(format, args...)
}

pkg/output/reporter.go (lines 57-59)

func (r *StructuredReporter) Infof(format string, args ...any) {
	r.inner.Infof(format, args...)
}

pkg/reporter/reporter.go (lines 51-56)

func (r *Object) Debugf(format string, args ...any) {
	if !debug.Enabled() {
		return
	}
	r.Infof(format, args...)
}

pkg/reporter/reporter.go (lines 59-66)

func (r *Object) Infof(format string, args ...any) {
	message := fmt.Sprintf(format, args...)
	if color.UseColor() {
		_, _ = fmt.Fprintf(os.Stdout, "%s%s\n", infoColorPrefix, message)
	} else {
		_, _ = fmt.Fprintf(os.Stdout, "%s%s\n", infoPrefix, message)
	}
}

pkg/reporter/reporter.go (lines 69-76)

func (r *Object) Warnf(format string, args ...any) {
	message := fmt.Sprintf(format, args...)
	if color.UseColor() {
		_, _ = fmt.Fprintf(os.Stderr, "%s%s\n", warnColorPrefix, message)
	} else {
		_, _ = fmt.Fprintf(os.Stderr, "%s%s\n", warnPrefix, message)
	}
}

pkg/reporter/reporter.go (lines 83-91)

func (r *Object) Errorf(format string, args ...any) error {
	message := fmt.Sprintf(format, args...)
	if color.UseColor() {
		_, _ = fmt.Fprintf(os.Stderr, "%s%s\n", errorColorPrefix, message)
	} else {
		_, _ = fmt.Fprintf(os.Stderr, "%s%s\n", errorPrefix, message)
	}
	return errors.New(message)
}

@olucasfreitas

Copy link
Copy Markdown
Contributor Author

@amandahla ready for review

@amandahla

Copy link
Copy Markdown
Contributor

@olucasfreitas please check the failures, you are facing same error that I got in my PR, 1.26.4 is not available in all images.
terraform-redhat/terraform-provider-rhcs#1240

@openshift-ci openshift-ci Bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 3, 2026
@olucasfreitas olucasfreitas changed the title ROSAENG-61053 | chore: bump Go to 1.26 ROSAENG-61053 | chore: bump Go to 1.26.3 Jul 3, 2026
Update the ROSA CLI toolchain from Go 1.25.8 to Go 1.26.4 across
go.mod, Dockerfiles, and Renovate configuration. Remove the
k8s.io/apimachinery < 0.36.0 version pin that was only necessary
while the repo was on Go 1.25.

Run go fix as part of the Go 1.26 upgrade and keep the verified
subset of mechanical updates that passes the repo's format, lint,
changed-lines coverage, and unit/integration test gates.

Paired with: openshift/release#81358
Follow the 1.26.3 rollout pattern used in terraform-provider-rhcs
and move the ROSA repo off 1.26.4, which is not available in all
required images yet. Align go.mod, Dockerfiles, Renovate, and the
committed bingo tool modules to the 1.26.3 baseline.
@olucasfreitas olucasfreitas force-pushed the ROSAENG-61053-go126 branch from aee8afd to 23ac5e9 Compare July 3, 2026 18:13
@openshift-ci openshift-ci Bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 3, 2026
@olucasfreitas

Copy link
Copy Markdown
Contributor Author

@amandahla updated to the 1.26.3 pattern from terraform-provider-rhcs#1240.

This branch now:

  • uses go 1.26.3
  • uses ubi9/go-toolset:1.26.3 in the Dockerfiles
  • keeps the already-verified go fix subset on the branch
  • rebases cleanly on current master so the image jobs can merge and run

Local make pre-push-checks is green on the rebased head. Waiting on the new Prow runs for commit 23ac5e93958e734378d9d738bab3f88ed689ac87.

@olucasfreitas

Copy link
Copy Markdown
Contributor Author

/test lint

@openshift-ci

openshift-ci Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

@olucasfreitas: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/security 23ac5e9 link false /test security

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants